-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
E2e test poc #116
E2e test poc #116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for starting this Shazad. Couple of suggestions. Keeping bunch of commands on package.json
would ease up the process locally and we can even invoke them on CI.
- npm run
setup
- would kick in and start necessary images - npm run
test
- would make sure kibana is running locally and starts and verifies the runner - move the root level
e2e
folder inside tests as we dont want to have mutiple root level directories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement, but I think the shell scripts may create more issues than they solve. Left the details of my concerns there in the comment with @jahtalab
I also left some comments on improving the shell scripts if indeed that is the way we want to go.
Another thought, we could use node
instead of bash to drive any orchestration. Shell gets unwieldy sometimes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its in a good shape, personally i would move this shell stuff to JS, but we can keep it for now and move to JS at some point to customize it.
Have not tested it, but will take it for a spin sometime this week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @shahzad31 , I left some comments.
Also would you please have a look at the merge check list, it is failing at the moment?
This is a POC for how we can make sure e2e things are working
You can test by running
npm run test inside __tests/e2e dir